Skip to content

Graphite tools#741

Open
99cardz wants to merge 12 commits intografana:mainfrom
99cardz:graphite-tools
Open

Graphite tools#741
99cardz wants to merge 12 commits intografana:mainfrom
99cardz:graphite-tools

Conversation

@99cardz
Copy link
Copy Markdown

@99cardz 99cardz commented Apr 13, 2026

Note

Medium Risk
Introduces new HTTP-proxy query paths and adds a fallback-transport behavior change that could affect datasource querying across deployments; coverage is improved with new unit/integration tests but the surface area is non-trivial.

Overview
Adds a new disabled-by-default graphite tool category, registering Graphite datasource tools (query_graphite, list_graphite_metrics, list_graphite_tags, query_graphite_density) with empty-result hints and both unit + integration coverage.

Extends the integration test environment to include a Graphite container + seeded metrics, provisions a Graphite datasource in Grafana, and makes CI waits deterministic (Grafana API readiness, datasource provisioning, and Graphite queryability). Also tightens datasourceFallbackTransport caching to only remember the fallback path on successful (2xx) responses.

Reviewed by Cursor Bugbot for commit 6f042df. Bugbot is set up for automated code reviews on this repo. Configure here.

99cardz added 3 commits April 12, 2026 10:52
- Implement GraphiteClient for handling queries to a Graphite datasource via Grafana proxy.
- Add functions for querying metrics, listing metrics, and retrieving tags.
- Enhance error handling and provide hints for empty results specific to Graphite.
- Include unit tests for Graphite functionalities to ensure reliability.
@99cardz 99cardz requested a review from a team as a code owner April 13, 2026 13:54
Comment thread tools/graphite.go Outdated
@cla-assistant
Copy link
Copy Markdown

cla-assistant bot commented Apr 13, 2026

CLA assistant check
All committers have signed the CLA.

Comment thread tools/graphite.go
Copy link
Copy Markdown
Collaborator

@sd2k sd2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the PR!

A few of things we'd require before we can merge additional tools:

  • since Graphite isn't a native datasource, it shouldn't be enabled by default (follow the pattern used by Clickhouse or Cloudwatch to see how)
  • we'll need integration tests, where we spin up a Graphite instance with data in docker-compose, provision a Graphite datasource in the provisioning file, and test the tools against the real Grafana instance.

Otherwise this looks good at a cursory glance, I'll give it a deeper review once we have those things added.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.

Reviewed by Cursor Bugbot for commit 1a5dccc. Configure here.

Comment thread tools/graphite.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants